Skip to content

feat(event cache): automatic back-pagination for unread counts, MVP#6344

Open
bnjbvr wants to merge 11 commits intomainfrom
bnjbvr/automatic-backpagination-unread-counts
Open

feat(event cache): automatic back-pagination for unread counts, MVP#6344
bnjbvr wants to merge 11 commits intomainfrom
bnjbvr/automatic-backpagination-unread-counts

Conversation

@bnjbvr
Copy link
Member

@bnjbvr bnjbvr commented Mar 23, 2026

This adds a very basic system for automatic back-paginations, with a way to trigger background requests from any component. At the moment it's super simple: it will only run one pagination request at a time, based on a maximum number of pagination credits per room (defaulted to 20). It's disabled by default, and there's a config toggle to enable it at the EventCache level; this toggle must be set before subscribing to the event cache (in particular, for consumers of the FFI layer, that means before creating a timeline or a room list service or a sync service).

One open question: this may leave room event caches in a state where a linked chunk has many events loaded in memory. This might be an issue, but since this is experimental, I'd rather land this piecemeal, rather than having the perfect implementation at start (because shrinking the linked chunk would cause paginations to happen just thereafter, if a read receipt points to an event that is far away enough that it'd be only loaded on disk, thereby causing an infinite loop of shrinking / reloading / shrinking etc.). Is that fine, for the moment?

Most of #6014.

  • I've documented the public API Changes in the appropriate CHANGELOG.md files.
  • This PR was made with the help of AI (autocomplete).

@bnjbvr bnjbvr requested a review from a team as a code owner March 23, 2026 16:46
@bnjbvr bnjbvr requested review from andybalaam and removed request for a team March 23, 2026 16:47
@Hywan Hywan self-requested a review March 23, 2026 16:48
@Hywan
Copy link
Member

Hywan commented Mar 23, 2026

One open question: this may leave room event caches in a state where a linked chunk has many events loaded in memory. This might be an issue, but since this is experimental, I'd rather land this piecemeal, rather than having the perfect implementation at start (because shrinking the linked chunk would cause paginations to happen just thereafter, if a read receipt points to an event that is far away enough that it'd be only loaded on disk, thereby causing an infinite loop of shrinking / reloading / shrinking etc.). Is that fine, for the moment?

I would personally go the other way: first address this problem, because it's not an easy one.

It could be part of the Strategy (see #6014) to decide whether the paginated events should land in-memory + in-store, or only in-store. What do you think?

@codecov
Copy link

codecov bot commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 96.25668% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.85%. Comparing base (47c7a20) to head (cf57e71).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...matrix-sdk/src/event_cache/caches/read_receipts.rs 85.71% 4 Missing and 2 partials ⚠️
crates/matrix-sdk/src/event_cache/tasks.rs 99.10% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main    #6344    +/-   ##
========================================
  Coverage   89.85%   89.85%            
========================================
  Files         376      376            
  Lines      102778   102931   +153     
  Branches   102778   102931   +153     
========================================
+ Hits        92352    92493   +141     
- Misses       6859     6870    +11     
- Partials     3567     3568     +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 23, 2026

Merging this PR will not alter performance

✅ 50 untouched benchmarks


Comparing bnjbvr/automatic-backpagination-unread-counts (cf57e71) with main (47c7a20)

Open in CodSpeed

@bnjbvr
Copy link
Member Author

bnjbvr commented Mar 23, 2026

One open question: this may leave room event caches in a state where a linked chunk has many events loaded in memory. This might be an issue, but since this is experimental, I'd rather land this piecemeal, rather than having the perfect implementation at start (because shrinking the linked chunk would cause paginations to happen just thereafter, if a read receipt points to an event that is far away enough that it'd be only loaded on disk, thereby causing an infinite loop of shrinking / reloading / shrinking etc.). Is that fine, for the moment?

I would personally go the other way: first address this problem, because it's not an easy one.

It could be part of the Strategy (see #6014) to decide whether the paginated events should land in-memory + in-store, or only in-store. What do you think?

I think it's actually less pragmatic to solve this hard problem now, because by having something available behind a feature flag that can be toggled dynamically at runtime, a feature which is already implemented in this PR, we can inform ourselves about what is missing in this implementation and learn otr more about the requirements and see how much of a problem in practice it is, before engineering a complex solution that may not be useful after all. (Note the solution isn't as simple as "do only in store vs do in-memory", because a subscription might happen while we're in the "do only in store mode".)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants